Skip to content

Enable similarity algorithms#17

Merged
IoannisPanagiotas merged 4 commits intomainfrom
enable-similarity-algos
Aug 19, 2025
Merged

Enable similarity algorithms#17
IoannisPanagiotas merged 4 commits intomainfrom
enable-similarity-algos

Conversation

@IoannisPanagiotas
Copy link
Copy Markdown
Collaborator

No description provided.

@IoannisPanagiotas IoannisPanagiotas force-pushed the enable-similarity-algos branch 17 times, most recently from 33ccc47 to 071d5e7 Compare August 14, 2025 13:32
@IoannisPanagiotas IoannisPanagiotas changed the title Support node similarity Enable similarity algorithms Aug 14, 2025
"description": "Property name to use for identifying nodes (e.g., 'name', 'Name', 'title'). Use get_node_properties_keys to find available properties.",
},
},
"required": ["sourceNodeFilter", "targetNodeFilter"],
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

},
},
"required": ["sourceNodeFilter", "targetNodeFilter", "nodeProperties"],
"required": ["nodeProperties"],
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the documentation is wrong!
if both are required, then that would mean you would need to specify both each time .e., you would not be able to add only source filter alone etc.

i think the docs are such that they reflect if you dont need to use them just go for normal similarity algo.

see also the tests in test_similarity.algos they test solo these calls etc.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then I think we can just have two tools, node_sim and kNN, and have the optional filters?
We could also "fix" the official doc.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fine by me

{
"nodeIdentifierProperty": "name",
"topK": 3,
"sourceNodeFilter": ["Acton Town"],
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but this test suggest without targetNodeFilter (which defaults to None in the code), it still works?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep

@IoannisPanagiotas IoannisPanagiotas marked this pull request as ready for review August 19, 2025 12:55
@IoannisPanagiotas IoannisPanagiotas force-pushed the enable-similarity-algos branch 5 times, most recently from 65cc3d2 to a59db85 Compare August 19, 2025 14:04
@IoannisPanagiotas IoannisPanagiotas merged commit b3c1162 into main Aug 19, 2025
1 check passed
@brs96 brs96 deleted the enable-similarity-algos branch September 10, 2025 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants